[Android] InputTransparent="true" on a Layout breaks child controls - Fix #22345
Conversation
|
Hey there @kubaflo! Thank you so much for your PR! Someone from the team will get assigned to your PR shortly and we'll get it reviewed. |
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
jsuarezruiz
added
platform/android 🤖
area-layout
|
We got a few issues with the changing container view that I tried to look at here #13114 I don't think my PR is right and needs to be done differently, but this container view issue is all over the properties. |
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
| handler.UpdateValue(nameof(IView.Visibility)); | ||
|
|
||
| #if WINDOWS | ||
| handler.UpdateValue(nameof(IView.Opacity)); |
There was a problem hiding this comment.
Do we know if it is just windows only? Somehow I feel opacity is such a basic feature it affects more and we just saw it with windows first.
If we don't want to move the cheese to much in the PR, maybe just make this comment an issue and I can have a look when I get back to my keyboard.
| @@ -417,6 +417,12 @@ public static void MapContainerView(IViewHandler handler, IView view) | |||
| handler.HasContainer = viewHandler.NeedsContainer; | |||
| else | |||
| handler.HasContainer = view.NeedsContainer(); | |||
|
|
|||
| handler.UpdateValue(nameof(IView.Visibility)); | |||
There was a problem hiding this comment.
Can we set this so it only fires if the value of HasContainer has changed?
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
| if (handler is ViewHandler viewHandler) | ||
| handler.HasContainer = viewHandler.NeedsContainer; | ||
| else | ||
| handler.HasContainer = view.NeedsContainer(); | ||
|
|
||
| if(hasContainerOldValue != handler.HasContainer) |
There was a problem hiding this comment.
I wonder if this should be a command? We can keep it "internal" for now, but just a command that is triggered when the container view changes? I think there are a few places that this is important to track:
Just looking at the Core layer in my PR: https://github.com/dotnet/maui/pull/13114/files#diff-e802e21561b0e99ffbb055b9ea43d833899fc9f06d91a64c5a9067c9427b7bcc there are several things I was adding to the ContainerAffectingProperties: IView.Background, ILabel.VerticalTextAlignment, IView.Visibility, IView.Opacity, IView.Clip, IView.Shadow, IBorder.Border, IView.InputTransparent. Basically, anything that would trigger a container view also needs to be re-triggered when the container view changes.
Just like you are fixing here, if the opacity changes, we need to trigger some logic so opacity can be correctly mapped to the correct view if the visibility happens and secretly adds a container. And the same is true if you add a container because of the opacity, the visibility of both the container and platform view is managed.
I think what you have here is much better than my PR - no need to collect everything in a list - but if the command fires, each handler can control what it does when the container changes.
@PureWeen not sure if this is just abusing the system even more and we really should just do it right with a method? Maybe the view handler can get an internal OnContainerViewChanged(PlatformView oldValue, PlatformView newValue)?
All we need is a way for a handler to decide to re-fire mappers if the container changes. Commands are nice as they can be extended via third parties without having to derive from the existing handler. But, will anyone ever extend the mappers? Maybe we can have an internal virtual for now and think about it some more and maybe move to a command later?
I don't think this PR should fix everything, I would really want to add the tests for that. But if this PR can setup the base/correct/current plan for doing this with the container view, then we can add new PRs to fix things.
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
Issues Fixed
Fixes #22289
Fixes #20264
Fixes #23344
bug.mov
fixed.mov